Skip to content

[ALICE3] Update ALICE3 centrality task - #17522

Merged
njacazio merged 5 commits into
AliceO2Group:masterfrom
axti98:Alice3CentralityDev
Aug 20, 2026
Merged

[ALICE3] Update ALICE3 centrality task#17522
njacazio merged 5 commits into
AliceO2Group:masterfrom
axti98:Alice3CentralityDev

Conversation

@axti98

@axti98 axti98 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor
  • Make CCDB timestamp configurable
  • Replace for loop and if statements with Filter + .size() call
  • Remove LOG call
  • Fix O2Linter errors

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

O2 linter results: ❌ 1 errors, ⚠️ 0 warnings, 🔕 0 disabled

@alibuild alibuild mentioned this pull request Aug 20, 2026
Merged
axti98 and others added 2 commits August 20, 2026 11:37
@axti98
axti98 marked this pull request as ready for review August 20, 2026 09:56
@alibuild

alibuild commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/code-check for 269f48c at 2026-08-20 12:19:

## sw/BUILD/O2Physics-code-check-latest/log
--
========== List of issues found ==========
++ echo 'Found 2 errors and 0 warnings.'
Found 2 errors and 0 warnings.
++ [[ 2 -gt 0 ]]
++ cat /sw/BUILD/db6337a40690b1d2e2d91ac889b5c052d23d61de/O2Physics-code-check/errors.txt
ALICE3/TableProducer/alice3Centrality.cxx:45:8: error: constructor does not initialize these fields: ccdb [cppcoreguidelines-pro-type-member-init,-warnings-as-errors]
ALICE3/TableProducer/alice3Centrality.cxx:50:16: error: no header providing "int64_t" is directly included [misc-include-cleaner,-warnings-as-errors]
++ [[ 0 -gt 0 ]]
++ [[ 2 -gt 0 ]]
++ exit 1
--

Full log here.

jesgum
jesgum reviewed Aug 20, 2026
View reviewed changes

@jesgum jesgum left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @axti98 ! Thanks a lot for the PR! I just have some minor suggestions regarding the code itself.
I also have a general comment regarding the calibration object that is currently in Analysis/ALICE3/Centrality. This looks to be created with v2 in mind (looks like |eta| < 4 is used), so it would probably have to be updated. Also, it is not clear to me, just from inspecting it, what generator was used (the calibration object would be different depending on the generator and the generator settings). Something to consider is if we should have some form of safeguard for this

Comment thread ALICE3/TableProducer/alice3Centrality.cxx Outdated
void init(InitContext&)
{
const AxisSpec axisMult{MaxMult.value > 10000.f ? 10000 : (int)MaxMult, 0, MaxMult, "Reconstructed tracks"};
const AxisSpec axisMult{maxMult.value > DefaultMult ? static_cast<int>(DefaultMult) : static_cast<int>(maxMult), 0, maxMult, "Reconstructed tracks"};

@jesgum jesgum Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be more convenient with ConfigurableAxis

Comment thread ALICE3/TableProducer/alice3Centrality.cxx Outdated
{
if (!centralityLoaded) {
hCumMultALICE3 = ccdb->getForTimeStamp<TH1D>("Analysis/ALICE3/Centrality", 1);
hCumMultALICE3 = ccdb->getForTimeStamp<TH1D>("Analysis/ALICE3/Centrality", ccdbNoLaterThan.value);

@jesgum jesgum Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to make the ccdb path a configurable as well, since it might be a bit more convenient if new calibrations are produced (it might be annoying to always try and upload them to Analysis/ALICE3/Centrality as everyone might not have the permissions and instead one could just use their own directory)

@axti98 axti98 Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jesper, yes I agree with your concerns. So the Object behind Analysis/ALICE3/Centrality is just the centrality percentile versus multiplicity, so I will make this path a configurable and upload my own calibration based on PYTHIA

@axti98
axti98 requested a review from jesgum August 20, 2026 15:16
@alibuild

alibuild commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/code-check for b448b3c at 2026-08-20 18:02:

## sw/BUILD/O2Physics-code-check-latest/log
--
========== List of issues found ==========
++ echo 'Found 1 errors and 0 warnings.'
Found 1 errors and 0 warnings.
++ [[ 1 -gt 0 ]]
++ cat /sw/BUILD/fa9286e6ccc789f8321d01ac27b4ca441ed1d81f/O2Physics-code-check/errors.txt
ALICE3/TableProducer/alice3Centrality.cxx:44:8: error: constructor does not initialize these fields: ccdb [cppcoreguidelines-pro-type-member-init,-warnings-as-errors]
++ [[ 0 -gt 0 ]]
++ [[ 1 -gt 0 ]]
++ exit 1
--

Full log here.

@jesgum

jesgum commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Approving from my side, leaving it for @njacazio to have a look and merge

@njacazio
njacazio merged commit 03d4b77 into AliceO2Group:master Aug 20, 2026
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jesgum jesgum jesgum approved these changes
@alibuild alibuild Awaiting requested review from alibuild alibuild is a code owner
@njacazio njacazio Awaiting requested review from njacazio njacazio is a code owner
@hscheid hscheid Awaiting requested review from hscheid hscheid is a code owner

Assignees

No one assigned

Labels

alice3

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@axti98 @alibuild @jesgum @njacazio